-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
11fa58e
to
ca4e7b5
Compare
Podspec URL will need to be updated to point to an iOS tag or some stable branch — it points to the current release branch now, as that’s the target of this PR. |
One potential pitfall of piggy-backing on the binary size metrics workflow is that we may not be willing/able to switch the branch of that workflow to point at the current release branch. Will that be an issue, @kkaefer? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're currently at ~25 min build runtime, so adding publishing should keep this well within the build timeout.
GENERIC_NIGHTLY_FILENAME="mapbox-ios-sdk-${PUBLISH_VERSION}.zip" | ||
aws s3 cp \ | ||
s3://mapbox/$REPO_NAME/ios/builds/${ZIP} \ | ||
s3://mapbox/$REPO_NAME/ios/builds/${GENERIC_NIGHTLY_FILENAME} --acl public-read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of copying over, can we create a link on S3 that points to the most recent version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far as I can tell, s3 doesn’t support symlinks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object-level redirects look like they're perfectly suited for this too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it may be possible to create a placeholder file and then the s3 API could be used here to set up a redirect to the latest version by updating the metadata of the placeholder with x-amz-website-redirect-location
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be ideal — I’ll look into setting up a redirect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn’t make redirects work — we’re not strictly a “website” and aws is simply passing the x-amz-website-redirect-location
header in GET requests, returning HTTP 200 and a blank file instead of HTTP 301 and the redirected file.
$ aws s3api put-object --bucket mapbox --key mapbox-gl-native/ios/builds/mapbox-ios-sdk-nightly-dynamic.zip --acl public-read --website-redirect-location /mapbox-ios-sdk-nightly-dynamic-2017-03-09.zip
$ curl -I -L https://mapbox.s3.amazonaws.com/mapbox-gl-native/ios/builds/mapbox-ios-sdk-nightly-dynamic.zip
HTTP/1.1 200 OK
x-amz-id-2: Ub2UZ8qnAymsFa8eQUxPQZakb9Pwfb1TspVyl1kvulVT32uN6oZZMv2pO13eKiA9B5yPoxQMSig=
x-amz-request-id: A66C36AF51BB9C8E
Date: Fri, 10 Mar 2017 20:50:27 GMT
Last-Modified: Fri, 10 Mar 2017 20:50:23 GMT
ETag: "d41d8cd98f00b204e9800998ecf8427e"
x-amz-website-redirect-location: /mapbox-ios-sdk-nightly-dynamic-2017-03-09.zip
Accept-Ranges: bytes
Content-Type: application/zip
Content-Length: 0
Server: AmazonS3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, HTTP 200 appears to be the intended behavior for this endpoint, per this thread:
The redirection functionality is only available for S3 static websites, which unfortunately means that you are seeing the intended behavior. A request served via the usual S3 endpoint will result in a response with the x-amz-website-redirect-location header, while the S3 website endpoint returns an HTTP redirect. Changing the origin to mybucketname.s3-website-us-east-1.amazonaws.com instead of using the bucket directly can theoretically solve the problem.
Hitting a website endpoint confirms we’re not set up as a website on s3. Configuring this bucket as a static website is more than I want to do to get this redirect working.
@friedbunny Ideally, the binary size metrics build will always run on master, not on other branches. |
platform/ios/INSTALL.md
Outdated
@@ -85,6 +85,14 @@ To test pre-releases and/or betas, you can reference the pre-release like so in | |||
pod 'Mapbox-iOS-SDK', podspec: 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/<insert branch or tag>/ios/Mapbox-iOS-SDK.podspec' | |||
``` | |||
|
|||
##### Testing nightly releases with CocoaPods | |||
|
|||
To test a nightly dynamic framework build, use this line to your Podfile: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix the typo here.
OK, then I think we’ll want to create another nightly workflow that can have its branch retargeted for releases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really glad this is happening! I don't have a strong opinion about the redirect (instead of symlink) optimization but it does seem like it may be possible.
In general, I think it'd make sense to target this PR at master instead of the release branch and use the nightly for a "nightly mainline?" build.
An additional, similar bitrise workflow could be set up to create nightly builds "nightly prerelease?" for our longer running major / minor release branches.
If we think that having two nightlies would be overkill then we could just do the second option only and manually point it back and forth from master <-> release branch as required.
platform/ios/INSTALL.md
Outdated
|
||
```rb | ||
pod 'Mapbox-iOS-SDK-nightly-dynamic', podspec: 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/release-ios-v3.5.0-android-v5.0.0/ios/Mapbox-iOS-SDK-nightly-dynamic.podspec' | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand how this would work, if a developer uses this, then they will reach the .podspec file on this temporary release branch on GitHub but that podspec has an s3 URL that points at an archive of a build from the master
branch (and that's assuming that zip archive does exist, which I don't think it would, until this change is merged back to master)
GENERIC_NIGHTLY_FILENAME="mapbox-ios-sdk-${PUBLISH_VERSION}.zip" | ||
aws s3 cp \ | ||
s3://mapbox/$REPO_NAME/ios/builds/${ZIP} \ | ||
s3://mapbox/$REPO_NAME/ios/builds/${GENERIC_NIGHTLY_FILENAME} --acl public-read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it may be possible to create a placeholder file and then the s3 API could be used here to set up a redirect to the latest version by updating the metadata of the placeholder with x-amz-website-redirect-location
.
d23c87e
to
f4d890c
Compare
Retargeted this against master and changed the docs to point there. I think we should merge this and then add a release branch nightly later, if we find that we want one. |
f4d890c
to
bce352b
Compare
Fixes #4196. Uploads a nightly build to s3 and makes it available via a special podspec.
Mapbox-iOS-SDK-nightly-dynamic
podspec that points to the latest nightly.Use this line in your podfile to test this branch:
To-do
Create new Bitrise workflow with Slack notifications, etc./cc @mapbox/mobile